fix(sanitization): secret exposure in function and agent trace spans - #6000
fix(sanitization): secret exposure in function and agent trace spans#6000BillLeoutsakosvl346 wants to merge 9 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@cursor review |
PR SummaryHigh Risk Overview Private provenance transport: Internal JWT callers can opt in via Execution route behavior: Workflow execute adds session-only Client/UI: The builder terminal and execution utils consume server Other: Functional block outputs for execution status use a shared collector with 409 when unavailable; background workflow/resume jobs distinguish core-finalized errors from faults that still need Reviewed by Cursor Bugbot for commit 0acc905. Configure here. |
Greptile SummaryThis PR stops resolved environment secrets from appearing in workflow trace spans and Overview logs while keeping runtime execution and API payloads unchanged.
Confidence Score: 5/5No prior Greptile findings remain to re-score and no eligible follow-up inline comments were identified, so the PR appears safe to merge from this pass. No blocking failure remains from outstanding prior threads or newly eligible incomplete-fix findings in this follow-up review.
|
| Filename | Overview |
|---|---|
| apps/sim/executor/utils/resolved-secret-trace-registry.ts | New catalog/active-match registry with encrypted provenance, scope checks, and bounded export-for-value filtering for cross-boundary secret tracking. |
| apps/sim/lib/logs/execution/trace-secret-projection.ts | Terminal projection that rewrites secret literals in trace trees (including large-value refs) without mutating runtime block state. |
| apps/sim/lib/logs/execution/logger.ts | Completes logs with prepared/projected spans, preserves private provenance on PII-masked execution state, and adjusts size compaction of tool-call content. |
| apps/sim/executor/variables/resolvers/env.ts | Records resolved env secrets into the trace registry only when the name exists on execution environment variables. |
| apps/sim/lib/workflows/executor/execution-core.ts | Wires registry creation and trace projection into the main workflow execution completion path. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
EnvResolve["EnvResolver.recordResolved"] --> Registry["ResolvedSecretTraceRegistry"]
Nested["MCP / function / mothership / nested WF"] --> Prov["Encrypted provenance import/export"]
Prov --> Registry
Runtime["Raw block I/O for execution"] --> Logs["buildTraceSpans / completeWorkflowExecution"]
Registry --> Project["projectTraceSpansForSecrets"]
Logs --> Project
Project --> Persist["Persisted executionData.traceSpans"]
Project --> Overview["Overview / export / UI logs"]
Reviews (5): Last reviewed commit: "secrets sanitization correctness" | Re-trigger Greptile
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 95d4fd1. Configure here.
|
@cursor review |
|
@greptile please review the latest commit, including the workflow-overview sanitization follow-up and the streaming token-accounting correction. |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit e73afbe. Configure here.
|
Demo: Screen.Recording.2026-07-28.at.12.00.01.AM.mov |
icecrasher321
changed the title
…zation-trace-spans # Conflicts: # apps/sim/lib/logs/execution/logging-session.ts
…zation-trace-spans
…zation-trace-spans
…zation-trace-spans
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 35187658 | Triggered | Username Password | 64f791f | apps/desktop/src/main/browser-credentials/vault.test.ts | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
| executionId, | ||
| workflowId, | ||
| data: { blockId, chunk }, | ||
| data: { blockId, chunk, display }, |
There was a problem hiding this comment.
SSE still sends raw stream text
High Severity
Workflow SSE stream:chunk and agent stream:thinking events still include the full raw chunk/text while projectLiveDisplayText may return only clearLiveDisplay or a sanitized display. Any SSE consumer (browser stream, reconnect, logging) can read resolved secrets even when the display copy is suppressed.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 0acc905. Configure here.
| body: JSON.stringify({ input, triggerType: 'manual', stream: true }), | ||
| body: JSON.stringify({ | ||
| inputFromExecutionId: executionId, | ||
| triggerType: 'manual', |
There was a problem hiding this comment.
Client ignores chunk display projection
Medium Severity
Log retry now sends only inputFromExecutionId, and the execute route sets input from stored workflowInput. Executions that predate persisted workflowInput load successfully but run with undefined input, unlike the removed client fallback that reconstructed input from starter block state.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 0acc905. Configure here.
| onBlockError: (data) => { | ||
| // Failures often skip stream:done — settle thinking/tool chrome here. | ||
| settleAgentStreamChrome(data.blockId, 'error') | ||
| agentStreamChrome.settleBlock(data.blockId, 'error') |
There was a problem hiding this comment.
Stream chunks ignore display projection
Medium Severity
Server stream:chunk events now include a sanitized display payload alongside raw chunk, but the workflow execution hook still buffers and forwards only data.chunk. That bypasses the new secret-safe streaming path while thinking deltas already honor display, so resolved secrets can still appear in chat streaming callbacks and in-memory chunk aggregation during server-side runs.
Reviewed by Cursor Bugbot for commit 0acc905. Configure here.
|
bugbot run |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
There are 5 total unresolved issues (including 3 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0acc905. Configure here.
| return NextResponse.json( | ||
| { ...payload, [RESOLVED_SECRET_PROVENANCE_FIELD]: provenance.exportProvenance() }, | ||
| { status: response.status, headers } | ||
| ) |
There was a problem hiding this comment.
MCP verify failure keeps success status
Medium Severity
When attachPrivateProvenance cannot read or parse the tool response (size limit, invalid JSON, etc.), it replaces the body with success: false but reuses the original HTTP status from the successful MCP call, so callers can see HTTP 200 alongside a failure payload.
Reviewed by Cursor Bugbot for commit 0acc905. Configure here.
| { status: 404 } | ||
| ) | ||
| } | ||
| input = sourceExecution.input |
There was a problem hiding this comment.
Retry omits legacy execution input
Medium Severity
Log retry now loads input only from persisted workflowInput via inputFromExecutionId. Executions that never stored that field still return found: true with missing input, so retry can start a run with undefined input instead of the previous starter/trigger payload.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 0acc905. Configure here.
2 participants
What changed
{{ENV_VAR}}references in trace/display values, including nested tool calls, timing segments, errors, object keys, and URL-encoded values.Root cause
Block inputs and provider/tool outputs were logged after environment references had been resolved. Function display code also resolved environment values, and post-executor paths could overwrite sanitized logs with raw streaming, pause-state, or workflow final-output data immediately before persistence.
Impact
New trace spans and Overview workflow output show configured references such as
{{AWS_SECRET_ACCESS_KEY}}instead of plaintext values. Runtime tool/function execution, API responses, downstream block behavior, billing, retries, pause/resume, and persisted runtime state remain unchanged. Existing historical logs are not rewritten.Validation
git diff --check